Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf(cubesql): Avoid allocations in MetaContext methods #9228

Merged
merged 11 commits into from
Feb 18, 2025
Merged

perf(cubesql): Avoid allocations in MetaContext methods #9228

merged 11 commits into from
Feb 18, 2025
+133 −108

Conversation

mcheshkov
Copy link
Member

Check List

  • Tests have been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made (if issue reference is not provided)

Selected results from my laptop:

power_bi_wrap           time:   [29.544 ms 29.578 ms 29.616 ms]
                        change: [-21.848% -21.635% -21.387%] (p = 0.00 < 0.05)

long_in_expr            time:   [20.256 ms 20.314 ms 20.348 ms]
                        change: [-4.7778% -4.3730% -4.0255%] (p = 0.00 < 0.05)

quicksight_1            time:   [9.3391 ms 9.3494 ms 9.3598 ms]
                        change: [-4.8924% -4.6718% -4.4695%] (p = 0.00 < 0.05)

large_model_300_select_wildcard
                        time:   [11.278 ms 11.365 ms 11.490 ms]
                        change: [-48.647% -48.052% -47.424%] (p = 0.00 < 0.05)

@codecov Codecov
Copy link

codecov bot commented Feb 17, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 86.84211% with 10 lines in your changes missing coverage. Please review.

Project coverage is 83.57%. Comparing base (2dd9a4a) to head (d00c068).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
rust/cubesql/cubesql/src/transport/ctx.rs 83.33% 8 Missing ⚠️
...besql/cubesql/src/compile/rewrite/rules/members.rs 66.66% 1 Missing ⚠️
...sql/cubesql/src/compile/rewrite/rules/old_split.rs 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #9228   +/-   ##
=======================================
  Coverage   83.57%   83.57%           
=======================================
  Files         227      227           
  Lines       81627    81615   -12     
=======================================
- Hits        68216    68207    -9     
+ Misses      13411    13408    -3
Flag Coverage Δ
cubesql 83.57% <86.84%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mcheshkov mcheshkov force-pushed the meta-context-avoid-allocs branch from 5c6b676 to 1d481c5 Compare February 17, 2025 22:16
@mcheshkov mcheshkov marked this pull request as ready for review February 17, 2025 22:18
@mcheshkov mcheshkov requested review from a team as code owners February 17, 2025 22:18
@mcheshkov mcheshkov force-pushed the meta-context-avoid-allocs branch from 1d481c5 to d00c068 Compare February 18, 2025 12:45
@mcheshkov mcheshkov merged commit ba753d0 into master Feb 18, 2025
76 checks passed
@mcheshkov mcheshkov deleted the meta-context-avoid-allocs branch February 18, 2025 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ovr ovr ovr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mcheshkov @ovr